Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support FCM changes #5411

Merged
merged 1 commit into from
Jul 31, 2024
Merged

Support FCM changes #5411

merged 1 commit into from
Jul 31, 2024

Conversation

norkans7
Copy link

@norkans7 norkans7 commented Jul 23, 2024

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (33f9bc2) to head (8926c1a).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #5411   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          504       504           
  Lines        25196     25204    +8     
=========================================
+ Hits         25196     25204    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@norkans7 norkans7 force-pushed the fix-fcm branch 3 times, most recently from aeb11ed to 39bf0e2 Compare July 24, 2024 18:05
@norkans7 norkans7 changed the title WIP support FCM changes Support FCM changes Jul 24, 2024
@@ -11,7 +13,15 @@ class ClaimView(ClaimViewMixin, SmartFormView):
class Form(ClaimViewMixin.Form):
title = forms.CharField(label=_("Notification Title"))
address = forms.CharField(
label=_("FCM Key"), help_text=_("The key provided on the the Firebase Console when you created your app.")
label="Project ID",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced the FCM key with project ID for now, we need something unique to use ad address

@norkans7 norkans7 marked this pull request as ready for review July 29, 2024 14:52
@@ -387,3 +387,8 @@ def prepare_value(self, value):

def valid_value(self, value):
return True


class PrettyJSONEncoder(json.JSONEncoder): # pragma: needs cover
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can go in temba.utils.json.PrettyEncoder and does it need # pragma: needs cover ?

@@ -57,6 +57,11 @@ def default(self, o):
return super().default(o)


class PrettyJSONEncoder(json.JSONEncoder):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other encoders/decoders in this file don't have JSON in the name because we typically import this module like from temba.utils import json

label=_("FCM Key"), help_text=_("The key provided on the the Firebase Console when you created your app.")
label="Key ID",
help_text=_("Private Key ID"),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird.. asking them to enter the private key ID that is in the JSON below. Why not just extract it in form_valid ?

@rowanseymour rowanseymour merged commit 2b6ce42 into main Jul 31, 2024
5 checks passed
@rowanseymour rowanseymour deleted the fix-fcm branch July 31, 2024 14:36
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants